Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update backup-restic to etcd 3.5.11 and fix addon documentation #2981

Merged
merged 1 commit into from
Dec 14, 2023
Merged

Update backup-restic to etcd 3.5.11 and fix addon documentation #2981

merged 1 commit into from
Dec 14, 2023

Conversation

embik
Copy link
Member

@embik embik commented Dec 14, 2023 •
edited
Loading

What this PR does / why we need it:
This updates the backup-restic addon to use etcd 3.5.11, the latest available 3.5.x image. Because the images are now using distroless, I have moved the parts that copy the certificates into the backup into the restic container before uploading the backup. Given that the container will have access to these files anyway (since its uploading them to s3), I did not see any gain from having another initContainer doing it.

In addition, while trying out the addon, I ran into a documentation issue - The s3Bucket param does not do what it's documented to do. If you just put a bucket name as is documented, the backups will be written to a local folder (because RESTIC_REPOSITORY is set to the contents of s3Bucket verbatim).

Now I had two choices, either fix the way RESTIC_REPOSITORY is set from s3Bucket or update documentation for the field. I have chosen to go with the latter, because anyone using the addon and validating their backups (which I hope everyone does ... right?) would have figured this out already. I'm afraid that breaking the existing way is going to create more issues (break it for everyone who already sets the s3:/ part).

I'm open to change this in a follow-up PR, but it wasn't the focus of this one so I just wanted to write down the as-is situation correctly.

Which issue(s) this PR fixes:

Fixes #2980

What type of PR is this?
/kind feature

Special notes for your reviewer:

Does this PR introduce a user-facing change? Then add your Release Note here:

Update backup-restic addon to use etcd 3.5.11 for creating etcd snapshots

Documentation:

https://github.com/kubermatic/docs/pull/1591

@embik
Update backup-restic to etcd 3.5.11
94dc0ea 
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
@kubermatic-bot kubermatic-bot added kind/feature Categorizes issue or PR as related to a new feature. release-note Denotes a PR that will be considered when it comes time to generate release notes. docs/none Denotes a PR that doesn't need documentation (changes). dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 14, 2023
@embik embik self-assigned this Dec 14, 2023
@xmudrii
Copy link
Member

xmudrii commented Dec 14, 2023

Now I had two choices, either fix the way RESTIC_REPOSITORY is set from s3Bucket or update documentation for the field. I have chosen to go with the latter, because anyone using the addon and validating their backups (which I hope everyone does ... right?) would have figured this out already. I'm afraid that breaking the existing way is going to create more issues (break it for everyone who already sets the s3:/ part).

I agree with going forward with the latter option. The former option would be a breaking change for sure.

xmudrii
xmudrii approved these changes Dec 14, 2023
View reviewed changes
Copy link
Member

@xmudrii xmudrii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm
/approve

addons/backups-restic/backups-restic.yaml
@@ -52,24 +52,14 @@ spec:
path: /etc/kubernetes/pki
initContainers:
- name: snapshoter
image: {{ Registry "gcr.io" }}/etcd-development/etcd:v3.5.6
image: {{ Registry "gcr.io" }}/etcd-development/etcd:v3.5.11
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to use k8s.gcr.io instead in the future.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should, KKP is also still using this image.

@kubermatic-bot kubermatic-bot added the lgtm Indicates that a PR is ready to be merged. label Dec 14, 2023
@kubermatic-bot
Copy link
Contributor

LGTM label has been added.

Git tree hash: f9ae99bda4ff23244cc119ce9224a57a2e042a40

@kubermatic-bot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: xmudrii

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@kubermatic-bot kubermatic-bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Dec 14, 2023
@xmudrii
Copy link
Member

xmudrii commented Dec 14, 2023

/cherrypick release/v1.7

@kubermatic-bot
Copy link
Contributor

@xmudrii: once the present PR merges, I will cherry-pick it on top of release/v1.7 in a new PR and assign it to you.

In response to this:

/cherrypick release/v1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@xmudrii
Copy link
Member

xmudrii commented Dec 14, 2023

/cherrypick release/v1.6

@kubermatic-bot
Copy link
Contributor

@xmudrii: once the present PR merges, I will cherry-pick it on top of release/v1.6 in a new PR and assign it to you.

In response to this:

/cherrypick release/v1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@embik embik mentioned this pull request Dec 14, 2023
Merged
@kubermatic-bot kubermatic-bot added docs/provided Denotes a PR that has a valid documentation reference. and removed docs/none Denotes a PR that doesn't need documentation (changes). labels Dec 14, 2023
@embik
Copy link
Member Author

embik commented Dec 14, 2023 •
edited
Loading

Re: s3 bucket name, this was accidentally changed in d751b26 (#1486). So it's a "regression" from 1.3 on, which means it was in this state for a long time.

@embik
Copy link
Member Author

embik commented Dec 14, 2023

/retest

guess that is a flake.

@kubermatic-bot kubermatic-bot merged commit 6b5c7b5 into kubermatic:main Dec 14, 2023
14 checks passed
@kubermatic-bot kubermatic-bot added this to the KubeOne 1.8 milestone Dec 14, 2023
@kubermatic-bot
Copy link
Contributor

@xmudrii: new pull request created: #2982

In response to this:

/cherrypick release/v1.7

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

This was referenced Dec 14, 2023
Merged
Merged
@kubermatic-bot
Copy link
Contributor

@xmudrii: new pull request created: #2983

In response to this:

/cherrypick release/v1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@embik embik deleted the update-restic-addon branch January 5, 2024 07:10
@xmudrii xmudrii added the backport-complete Denotes a PR or issue which has been fully backported to all required release branches. label Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xmudrii xmudrii xmudrii approved these changes

Assignees

@embik embik

@xmudrii xmudrii

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. backport-complete Denotes a PR or issue which has been fully backported to all required release branches. dco-signoff: yes Denotes that all commits in the pull request have the valid DCO signoff message. docs/provided Denotes a PR that has a valid documentation reference. kind/feature Categorizes issue or PR as related to a new feature. lgtm Indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Milestone
KubeOne 1.8
Development

Successfully merging this pull request may close these issues.

Update etcd image in backups-restic addon to distroless versions
3 participants
@embik @xmudrii @kubermatic-bot